-
Notifications
You must be signed in to change notification settings - Fork 0
feat: use junction instead of symlink for Windows #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Yeah, splitting them into two would be better. Also, could you change the target branch to c0rychu:support-windows, which makes it easier for me. |
c0rychu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments
5d11532 to
804523d
Compare
Done. I only keep the junction update for this PR. Will add the tests adaptation in another. |
c0rychu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ldkv thank you so much for your work!
- Sorry for being a little picky and opinionated here, but these are my comments/suggested changes.
- If the Windows-only test that mocks the
except OSErrorrequires too much work, we can merge this PR first and create that test later as long as you verify that it's working on your Windows machine. - It would be nice to add docstrings with Google style, but I can also ask
codex-clito do that afterwards, so either way, don't worry about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
c0rychu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address comments from cursor bugbot
Hey no worry, your product your call. Your comments are good too. I will let you update the docstrings as you like, personally I find the type annotations suffice. I will add a test for the OSError in the 2nd PR. |
c0rychu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great!
|
@cursor review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR replaces
symlinkwith Windows junction on Windows OS.It allows to link a directory on Windows without admin privileges or Developer Mode. Should address #5
Note
Adds cross-platform link utilities that create Windows junctions as needed and updates CLI, project logic, and tests to recognize them.
src/uvlink/path_utils.pywithis_windows,is_link_or_junction,path_exists,create_windows_junction, andcreate_symlink(fallback to junctions on Windows).src/uvlink/cli.py):linkto usepath_existsfor existence checks andcreate_symlinkfor creation; streamline prompts and creation flow.src/uvlink/project.py):rm_rfto remove links viais_link_or_junction.is_linkedusingis_link_or_junctioninProjects.get_list.is_symlink() or is_junction()), add non-existence check for junctions in dry-run, and usecreate_symlinkin project path resolution tests.unlink(missing_ok=True).Written by Cursor Bugbot for commit 4b04326. This will update automatically on new commits. Configure here.